-
Notifications
You must be signed in to change notification settings - Fork 993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Events in DocumentedObservation #3367
Conversation
@@ -46,7 +47,7 @@ class SimpleObservation implements Observation { | |||
|
|||
private final Collection<ObservationFilter> filters; | |||
|
|||
SimpleObservation(String name, ObservationRegistry registry, Context context) { | |||
SimpleObservation(@Nullable String name, ObservationRegistry registry, Context context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that this can be null here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I mean look at line 60, we literally null
the name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get that a DocumentedObservation
's getName()
can return null
for filtering purposes but if I take a step back, that might be valid for the DocumentedObservation
but I think SimpleObservation
should not accept null
here. What should the meter handler or the tracing handler do in that case?
Also, DocumentedObservation
has a null-check so that nullability behavior can be isolated there I guess.
micrometer-commons/src/main/java/io/micrometer/common/Event.java
Outdated
Show resolved
Hide resolved
micrometer-commons/src/main/java/io/micrometer/common/Event.java
Outdated
Show resolved
Hide resolved
micrometer-commons/src/main/java/io/micrometer/common/Event.java
Outdated
Show resolved
Hide resolved
bc9caf5
to
e358cd5
Compare
@cppwfs this might break task cause afair you're using events already. You'll need to change |
No description provided.